Skip to content

fix(desktop): show cached display names on startup - #3317

Open
TheSentinel454 wants to merge 1 commit into
block:mainfrom
TheSentinel454:tornquist/display-name-cache
Open

fix(desktop): show cached display names on startup#3317
TheSentinel454 wants to merge 1 commit into
block:mainfrom
TheSentinel454:tornquist/display-name-cache

Conversation

@TheSentinel454

Copy link
Copy Markdown
Contributor

Why

Buzz restores cached channels and messages before profile lookups complete. On launch, that briefly exposes pubkey-derived labels in place of familiar display names.

What

  • Persist a bounded, relay-scoped cache of last-known display names, NIP-01 names, and NIP-05 handles
  • Seed batch profile queries from those labels immediately, while keeping them stale so the existing relay request revalidates them
  • Keep cached data presentation-only: avatars and ownership metadata are not persisted or used to seed profile-detail caches
  • Remove cleared or missing profiles, purge a relay's labels when its community is removed, and include the cache in local-storage quota recovery
  • Add unit coverage for parsing, bounds, eviction, malformed data, and cleared profiles
  • Add an E2E regression that delays the relay profile response and verifies the cached name is rendered first

Risk Assessment

Low. The cache is disposable, capped at 1,000 entries per relay, scoped by normalized relay URL, and always revalidated. It contains only public label fields and does not restore avatars, agent ownership, or authorization state.

Verification

  • just ci
  • pnpm typecheck
  • pnpm test — 3,727 passed
  • pnpm exec playwright test tests/e2e/channels.spec.ts --grep "cached profile labels" — passed

Generated with Codex

@TheSentinel454

Copy link
Copy Markdown
Contributor Author

🤖 Before / after with the relay profile response deliberately delayed:

Before After
Before: pubkey-derived label After: cached display name

@TheSentinel454
TheSentinel454 marked this pull request as ready for review July 28, 2026 15:01
@TheSentinel454
TheSentinel454 requested a review from a team as a code owner July 28, 2026 15:01

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 thanks for this — it fills a real gap (the persisted snapshots don't store display names at all, so restored messages show pubkey labels until profiles resolve). The design is careful: presentation-only fields, always-stale revalidation, purge on community removal, quota-recovery integration. One behavioral thing worth fixing inline below, plus a few small nits.

Comment on lines +380 to +383
initialData: relayUrl
? () => readCachedUserLabels(relayUrl, normalizedPubkeys)
: undefined,
initialDataUpdatedAt: 0,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 I think initialData here silently defeats the keepPreviousData right above it. In TanStack Query, placeholderData only applies when a query has no data — and once initialData seeds the new query, it counts as real data. So in the exact case the comment above describes (scroll-back grows the pubkey set → brand-new query key), any label-cache hit means the new query starts from label stubs (avatarUrl: null) instead of the previous batch's full profiles. Two symptoms from that one cause:

  1. Message avatars blank on scroll-back until the fetch completes — a single frame when the per-pubkey entry cache is fresh, but a full network round-trip when entries are >60s old. That partially reintroduces the flash Fix display names flashing to pubkeys when loading older messages #1137 fixed, in avatar form.
  2. ChannelScreen gates DM huddle-member resolution on isPending || isPlaceholderData; with initialData the query is instantly success and not placeholder, so the gate opens while only label stubs are loaded. Fail direction is benign (ownership absent, never wrongly granted), but the gate's intent is defeated.

Suggested fix — deliver the cache through the placeholder chain instead:

placeholderData: (prev) => prev ?? readCachedUserLabels(relayUrl, normalizedPubkeys),

and drop initialData/initialDataUpdatedAt. That preserves keepPreviousData semantics (previous full profiles win over label stubs), keeps isPlaceholderData true so the ChannelScreen gate behaves as before, still triggers the revalidating fetch, and the dataUpdatedAt === 0 seeding guard below still holds since placeholder data never advances dataUpdatedAt. Your new e2e test should pass unchanged. Bonus: the localStorage read+parse gets skipped whenever previous data exists instead of running on every scroll-back key growth.

Comment on lines +490 to +496
test.beforeEach(async ({ page }, testInfo) => {
await installMockBridge(
page,
testInfo.title.includes("cached profile labels")
? { usersBatchDelayMs: 10_000 }
: undefined,
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 nit: injecting the delay by string-matching the test title is a bit fragile — if the test gets renamed the delay silently stops applying and the test would still pass without proving anything. A tag or a per-test fixture would be sturdier. Not blocking.

Comment on lines +361 to +363
if (relayUrl) {
writeCachedUserLabels(relayUrl, fresh.profiles, fresh.missing);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 nit: this writes labels under the relayUrl captured at render, so a fetch resolving across a community switch could in theory file profiles under the old relay's cache key. In practice the communityKey remount tears the query down first, so I think this is theoretical — just noting it.


type UserLabelCache = {
version: 1;
updatedAt: number;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 nit: the top-level updatedAt on the cache envelope is written and parsed but never consumed — the per-entry updatedAt does all the work. Could drop it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants